[SYCL] Fix one more bug in int-header generation #20706
Conversation
When free function kernel defined as a template has an argument whose type is an alias, we need to look into the underlying type, or otherwise we emit nonsense.
AlexeySachkov
temporarily deployed
to
WindowsCILock
GitHub Actions
Inactive
| // CHECK: void baz_type(ns::Bar<double, float> Arg); | ||
|
|
||
| #if 0 | ||
| // This test case fails, but it is added here in advance to add a record of a |
There was a problem hiding this comment.
Note for reviewers: this is an additional test case that I came up with, it was not (at least yet) reported by any customer. I do have a work-in-progress patch for it, but it requires bigger changes to address.
I'm adding the known failing test case right now, proceeding with the simpler patch to unblock the reporter. The extra issue that I found will be addressed in a separate PR
AlexeySachkov
temporarily deployed
to
WindowsCILock
GitHub Actions
Inactive
AlexeySachkov
temporarily deployed
to
WindowsCILock
GitHub Actions
Inactive
AlexeySachkov
temporarily deployed
to
WindowsCILock
GitHub Actions
Inactive
| } | ||
|
|
||
| const TemplateSpecializationType *TSTAsNonAlias = | ||
| TST->getAsNonAliasTemplateSpecializationType(); |
There was a problem hiding this comment.
Does that work for nested aliases? i.e. alias for alias?
There was a problem hiding this comment.
Just a note here. Nested alias is needed for pytorch. Please check below code from https://github.com/pytorch/pytorch/blob/a7f3b10866098c452d89cd7a30bc4ce5713b8319/aten/src/ATen/core/TensorAccessor.h#L45
template<typename T, size_t N, template <typename U> class PtrTraits = DefaultPtrTraits, typename index_t = int64_t>
using TensorAccessor = torch::headeronly::detail::TensorAccessor<c10::IntArrayRef, T, N, PtrTraits, index_t>;
...
template<typename T, size_t N, template <typename U> class PtrTraits = DefaultPtrTraits, typename index_t = int64_t>
using GenericPackedTensorAccessor = torch::headeronly::detail::GenericPackedTensorAccessor<TensorAccessor<T, N-1, PtrTraits, index_t>, detail::IndexBoundsCheck<N, index_t>, T, N, PtrTraits, index_t>;
There was a problem hiding this comment.
Does that work for nested aliases? i.e. alias for alias?
Yes, looking at the implementation it unwraps all aliases, I've also added a test case in 99174d6
AlexeySachkov
temporarily deployed
to
WindowsCILock
GitHub Actions
Inactive
AlexeySachkov
temporarily deployed
to
WindowsCILock
GitHub Actions
Inactive
AlexeySachkov
had a problem deploying
to
WindowsCILock
GitHub Actions
Failure
AlexeySachkov
had a problem deploying
to
WindowsCILock
GitHub Actions
Failure
|
The second commit only altered clang tests, so I will treat all failures in the second CI run as unrelated. In the first CI run the following tests failed: However, they don't use free function kernels and all this patch does is change how we generate integration header for free function kernels. As such, I'm going to treat these two as unrelated failures as well and proceed with merge |
AlexeySachkov
deleted the
private/asachkov/one-more-fix-for-aliases-and-free-function-kernels
branch
3 participants
When free function kernel defined as a template has an argument whose type is an alias, we need to look into the underlying type, or otherwise we emit nonsense.